feat(table): per-block/table version-range metadata for scan skipping#2304
Open
shaunpatterson wants to merge 4 commits into
Open
feat(table): per-block/table version-range metadata for scan skipping#2304shaunpatterson wants to merge 4 commits into
shaunpatterson wants to merge 4 commits into
Conversation
Add an inclusive [minVersion,maxVersion] version range to every BlockOffset and a min_version to TableIndex (max_version already existed), appended to the FlatBuffers schema. Appending keeps the format forward/backward compatible: old readers ignore the new fields and tables written by older code decode them as absent. Also append the near-free uncompressed_len + key_count to BlockOffset in the same schema bump. flatc was unavailable, so fb/TableIndex.go and fb/BlockOffset.go were hand-edited following the project's prior prefix-bloom precedent: new readers gate on rcv._tab.Offset(vtableOffset) != 0 (vtableOffset = 4 + 2*slot), StartObject counts bumped (TableIndex 7->8, BlockOffset 3->7), and matching Add*/Mutate* builder helpers added. Presence (not value) gates skipping, so a block whose true min/max are both 0 is conservatively treated as unknown and never pruned. The builder tracks a conservative per-block and per-table version range over every key's parsed ts, including tombstones (their version is in the key), so the range is always an over-approximation. A new IteratorOptions.UntilTs gives an inclusive upper bound complementing SinceTs, defining the window (SinceTs, UntilTs]. At table-pick time a table whose range falls entirely above UntilTs is skipped; the table Iterator skips whole blocks outside the window without decompressing them. Tables/blocks lacking range metadata are never skipped. The skip is a pure performance prune; the per-entry version filter (now also honoring UntilTs) stays authoritative. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGkC4K2J2XYwcAKwjhHbM
versionWindow() computed lower=SinceTs+1 unconditionally, but the authoritative per-entry filter only excludes v<=SinceTs when SinceTs>0. On an UntilTs-only scan (SinceTs==0), version-0 entries are in-window, yet the storage prune used lower=1 and would drop any block/table whose maxVersion==0 (0 < 1), silently omitting version-0 data. Set lower=0 when SinceTs==0 (the smallest version the per-entry filter keeps), else SinceTs+1. Also guard SinceTs==MaxUint64: SinceTs+1 wraps to 0, so return an empty window (lower=MaxUint64, upper=0) which prunes every block, matching the per-entry filter that keeps nothing in that case. Route the table-level pickTable/pickTables prune through versionWindow so both layers use identical bounds. Regression tests: TestVersionWindowLowerBound (boundary table incl. overflow), TestUntilTsKeepsVersionZeroTable (table picker keeps a [0,0] table on an UntilTs-only scan), table/TestVersionZeroBlockNotSkipped (version-0 block not skipped on a lower=0 window), and TestUntilTsLowerBoundKeepsSmallestVersion (end-to-end: badger rejects CommitTs==0, so the smallest storable version 1 survives an UntilTs-only scan after flush). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtGkC4K2J2XYwcAKwjhHbM
The reverse non-AllVersions walk-up loop advanced to a newer version whenever nextTs <= readTs, ignoring the UntilTs upper bound. With versions 3,4,7 and UntilTs=4, a reverse scan climbed past the window and returned v7, while the forward path correctly returns v4 (the per-entry filter drops v7). So a reverse scan disagreed with forward, and could return a version outside the requested (SinceTs, UntilTs] window. Gate the walk-up on UntilTs as well. SinceTs needs no re-check: candidates climb in increasing ts, so any version newer than the in-window entry is also > SinceTs. Adds TestUntilTsReverseReturnsInWindowVersion (reverse+forward parity); the existing window test uses AllVersions, which returns before this loop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Block-level version skipping (SetVersionBounds) can leave a table iterator invalid after Rewind when every block falls outside the window (seekToFirst/seekToLast set io.EOF). ConcatIterator.Rewind only positioned on the first table without looping, so if that table was fully pruned the ConcatIterator — and its MergeIterator level — looked exhausted and stranded every later table, silently dropping in-window entries (reachable at L1+ where a level uses a ConcatIterator; e.g. a table with old keys in early blocks and recent keys in later blocks, scanned with a mid version window). Add skipInvalidTables(), called from Rewind, mirroring the empty-table loop already in Next(). No-op when version bounds are disabled. Seek is unaffected: seekFrom does not version-skip blocks and the binary search guarantees the chosen table holds a key >= the seek key. Adds TestConcatIteratorSkipsFullyPrunedFirstTable (fails without the fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Today a table carries only a per-table
max_versionand a table-level lower bound viaSinceTs. There is no per-block version metadata and no upper bound, so a version-bounded scan (e.g.SinceTs-based incremental backup, or reading only versions at/below a known timestamp) must decompress and walk version sprawl it will then discard.What changed
BlockOffsetgainsmin_version/max_version(and the near-freeuncompressed_len/key_count);TableIndexgainsmin_version. The builder tracks these over every key (deletes included — their ts is in the key).IteratorOptions.UntilTs— an inclusive upper version bound, complementingSinceTsto define a window(SinceTs, UntilTs].[minVersion, maxVersion]does not intersect the active window.The flatbuffer fields are appended (forward/backward compatible): old readers ignore them, and new readers treat a table written without them as "range unknown" and never skip it.
Compatibility & correctness
The skip is a conservative over-approximation and a pure prune — the per-entry version filter remains authoritative. A block/table is skipped only when it provably contains no version in the window. The window lower bound is aligned exactly with the per-entry filter's keep boundary (
lower = 0whenSinceTs == 0, elseSinceTs+1), so anUntilTs-only scan cannot wrongly skip a low-version entry;SinceTs == MaxUint64is guarded against overflow. Default behavior (no version bounds set) is unchanged.Testing
go build,go vet, fullgo test . -count=1green. Tests cover: bounded-scan results identical with vs. without the skip across multiple windows incl. deletes; old-format tables never skipped; blocks/tables provably outside the window actually pruned; the version-window lower-bound boundary cases.🤖 Generated with Claude Code
https://claude.ai/code/session_01NtGkC4K2J2XYwcAKwjhHbM